Skip to content

some cleanup - #123

Closed
richard-burhans wants to merge 1 commit into
mainfrom
update-egapx
Closed

richard-burhans wants to merge 1 commit into
mainfrom
update-egapx

Conversation

@richard-burhans

Copy link
Copy Markdown
Owner

FOR CONTRIBUTOR:

  • I have read the CONTRIBUTING.md document and this tool is appropriate for the tools-iuc repo.
  • License permits unrestricted use (educational + commercial)
  • [] This PR adds a new tool or tool collection
  • This PR updates an existing tool or tool collection
  • This PR does something else (explain below)

@richard-burhans

Copy link
Copy Markdown
Owner Author

Closing as superseded — three of the four changes here have landed by other routes, and the branch conflicts with main.

this PR proposes on main today
@TOOL_VERSION@ 0.5.0 → 0.5.1, suffix → 0 0.5.2, suffix 0 — ahead of this PR
attribute reordering (size, multiple, area, collection_type moved last) already exactly that form
<environment_variable> re-indent already done
expect_test_failure="true""false" on two tests ⛔ still "true"

The reordering and indentation are presumably the canonical-form check enforcing them since; the version has been bumped twice past this branch.

The one thing that never landed is the expect_test_failure flip, and it deserves its own PR rather than riding along with cleanup. expect_test_failure="true" asserts this test is expected to fail; the egapx tools run the quay.io/galaxy/egapx container and pull SRA data, which CI cannot do. Flipping it to false is a claim about the environment, not a tidy-up, and it would turn those tests red in Test tools if the claim is wrong.

⚠ Worth noting if it is revisited: main now has four of these, not the two this PR touched —

tools/ncbi_egapx/ncbi_egapx.xml:369
tools/ncbi_egapx/ncbi_egapx.xml:391
tools/ncbi_egapx/ncbi_egapx_execute.xml:85
tools/ncbi_egapx/ncbi_egapx_prepare_input.xml:332

so a partial flip would leave the set inconsistent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant